Skip to content

Week4#8

Open
tunaunnie wants to merge 11 commits intodevelopfrom
week4
Open

Week4#8
tunaunnie wants to merge 11 commits intodevelopfrom
week4

Conversation

@tunaunnie
Copy link
Contributor

@tunaunnie tunaunnie commented Nov 15, 2024

Related issue 🛠

Work Description ✏️

  • MVVM 구조 적용
  • email 입력 -> userName 입력 방식으로 수정
  • 네트워크 관련 DTO 정의 및 파일 세팅
  • 전역적으로 유저 정보를 저장할 localDataSource 생성
  • 회원가입, 로그인, 취미 조회 API 연결 완료

Screenshot 📸

SOPT.week4.mp4

Uncompleted Tasks 😅

  • HomeScreen에서 TopBar 두 개 사이에 이상한 여백이 들어가는데 어디서 들어오는건지 모르겠습니다 ㅜㅜ Scaffold에서 innerpadding 제거하고자 할 때 innerPadding 자리를 그냥 it 으로 받고 사용하지 않는 방식으로 구현했는데, 노란 경고줄이 떠서.. 이렇게 해도 괜찮은지 궁금합니다
  • Mypage에 접근했을 때 취미&유저네임 정보가 깜빡깜빡하면서 로드가 됐다가 안됐다가 하는 현상이 발생합니다.. 어떨 때는 잘 되고 어떨 때는 첨부한 동영상처럼 클릭할 때만 잠깐 나왔다가 없어지는 이슈가 있습니다

To Reviewers 📢

@tunaunnie
Copy link
Contributor Author

ㅜㅜ 이번에도 과제 수행도가 많이 미흡하네요... 차주 내로 솔직한 반성 & 빠른 복구에 들어가겠습니다🥹

Copy link
Member

@t1nm1ksun t1nm1ksun left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

과제 브랜치 파실 때 이전 주차 브랜치 develop에 머지하고 develop에서 파주세요..!!
대부분의 file들이 Files changed로 인식됩니다 ㅠㅠ

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

DTO작성이 매우 자세하네요👍 래퍼클래스를 넣은 이유가 있나요? 궁금해서!!!

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

서버 요청이 성공한 경우와 실패한 경우에 사용하는 DTO가 분리되어 있는데, 응답 타입은 늘 하나여야 하니.. 분기 처리를 하는 것보단 WrapperDTO 안에 failedDTO랑 successDTO를 함께 넣어두고 늘 WrapperDTO를 반환하는 방식이 맞다고 생각했어요! 그런데 합세 때 재민님께 꼭 그렇게 하지 않아도 된다는 말씀을 들어서 (그리고 이게 지금 에러의 원인 같아서 😅 방식을 수정해볼 예정입니당)

import org.sopt.and.model.dto.signup.RequestCreateUserDto
import org.sopt.and.model.dto.signup.ResponseCreateUserSuccessDto
import org.sopt.and.model.dto.signup.ResponseCreateUserWrapperDto
import retrofit2.Call

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

저도 이번에 레트로핏2를 공부하면서 Call vs Response에 대해서 찾아봤어요.
차이점을 한번 찾아보시고 나중에 코루틴 적용에 Response가 더 이점이 있다고 하니 바꿔보시는것도 좋을 것 같아요!

Log.e(TAG, "유저 로그인 에러 ${response.code()}")
}
}

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

로그인 실패에 대해 더 자세히 에러처리가 있으면 좋을 것 같아요

}
}

fun PasswordValidCheck(password: String): Boolean {

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

이제는 로그인 명세를 보시면 8자 이하면 되기 때문에 없애줘도 될 것 같아요!

Copy link

@angryPodo angryPodo left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

이번에 세미나 참석 못해서 이해하는데 어려움이 있었을텐데 잘하셨어요. 합세 진행하면서 이번 리뷰에 달린 내용들이나 로직분리와 같은 것들 다시한번 적용하면 확 와닿을 것 같아요! 빠이팅!!

Copy link

@chattymin chattymin left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

이번주도 고생하셨습니다!
합세 시즌인데 화이팅이에요 :)

Comment on lines +67 to +71
var userNameState = loginViewModel.userNameState.collectAsState().value
var passwordState = loginViewModel.passwordState.collectAsState().value
var isUserNameValid = loginViewModel.isUserNameValid.collectAsState().value
var isPasswordValid = loginViewModel.isPasswordValid.collectAsState().value
var shouldShowPassword = loginViewModel.shouldShowPassword.collectAsState().value

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

collectAsState에는 단점이 하나 있습니다.
정확히는 flow의 단점이라고 할 수 있겠네요.

liveData와 flow의 차이점을 공부해보시고, collectAsStateWithLifecycle을 사용하시면 좋을 것 같습니다 :)

Comment on lines +19 to +20
//현재 로그인한 토큰으로 현 사용자의 취미를 불러오기 위함
private val userService by lazy { ServicePool.userService }

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

이렇게 선언하는 것과 viewModel의 인자로 선언하는 것의 차이점을 찾아보시면 좋을 것 같아요

Copy link
Contributor

@jihyunniiii jihyunniiii left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

작업 전 이전 주차 과제 PR들 머지하고 진행해주시고, 시연 영상 필수로 추가해주세요.
과제 제출하실 때 노션에 있는 과제 제출 방식 전부 따라주셔야 하고 과제에 있는 기능 명세 전부 구현해주셔야 합니다.
필수 과제는 정말 필수적인 부분이고 제대로 진행되지 않으면 추후 앱잼 등 협업 진행이 어려우니 시간 나실 때 꼭 수정 부탁드릴게요.
필수 과제가 제대로 진행되지 않을 시 앱잼 참여가 어려울 수 있습니다.

Comment on lines +6 to +12
@Serializable
data class ResponseGetUserWrapperDto(
@SerialName("success")
val success: ResponseGetUserSuccessDto,
@SerialName("failed")
val failed: ResponseGetUserFailDto
) No newline at end of file
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

이거 이런 식으로 만들어도 통신 잘 이루어 지나요?
성공의 경우에는 success에 대한 것만 실패의 경우에는 failed 만 내려보내주어 직렬화가 잘 안 될 것 같은데 잘 동작하나요?


@GET("/user/my-hobby")
fun getMyHobby(
@Header("token") request: RequestGetUserHobbyDto
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

이렇게 넣지 않고 인터셉터를 통해 넣는 방식도 있으니 나중에 공부해보시면 좋을 것 같아요!

}
}

override fun onFailure(call: Call<ResponseGetUserWrapperDto>, t: Throwable) {
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

onFailure 블럭은 어떤 경우에 들어오게 될까요?

Comment on lines +47 to +51
@Serializable
data class LoginScreen(
val userNameText: String,
val passwordText: String
)
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

어떨 때 사용되는 친구인가요?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

week4

5 participants